refactor: cleanup match ticker step 2#7693
Draft
Rathoz wants to merge 4 commits into
Draft
Conversation
Rathoz
changed the title
Collaborator
|
this throws (Module:MatchTicker/Custom/dev/hjp at line 62: attempt to call upvalue 'MatchTicker' (a table value).) |
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the commons MatchTicker implementation to separate match fetching/config parsing from widget rendering, and updates consuming infoboxes/wiki customizations to use the new API and return types.
Changes:
- Replaces the class-based
MatchTickerAPI (:query():create()) with a controller-style module plus standalone widget components. - Introduces new ticker widget components (
Wrapper,HorizontalContainer) and extracts the filterable container into its own module (withContainerdeprecated). - Updates infoboxes and multiple wiki
CustomPlayer:createBottomContent()annotations/call sites to align withRenderable?output.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lua/wikis/wildrift/Infobox/Person/Player/Custom.lua | Updates bottom content return annotation to Renderable?. |
| lua/wikis/valorant/Infobox/Person/Player/Custom.lua | Updates bottom content return annotation to Renderable?. |
| lua/wikis/stormgate/Infobox/Person/Player/Custom.lua | Updates bottom content return annotation to Renderable?. |
| lua/wikis/starcraft2/Infobox/Person/Player/Custom.lua | Updates bottom content return annotation to Renderable?. |
| lua/wikis/rainbowsix/Infobox/Person/Player/Custom.lua | Updates bottom content return annotation to Renderable?. |
| lua/wikis/overwatch/Infobox/Person/Player/Custom.lua | Updates bottom content return annotation to Renderable?. |
| lua/wikis/marvelrivals/Infobox/Person/Player/Custom.lua | Updates bottom content return annotation to Renderable?. |
| lua/wikis/leagueoflegends/Infobox/Person/Player/Custom.lua | Updates bottom content return annotation to Renderable?. |
| lua/wikis/geoguessr/Infobox/Person/Player/Custom.lua | Updates bottom content return annotation to Renderable?. |
| lua/wikis/dota2/Infobox/Person/Player/Custom.lua | Updates bottom content return annotation to Renderable?. |
| lua/wikis/brawlstars/Infobox/Person/Player/Custom.lua | Updates bottom content return annotation to Renderable?. |
| lua/wikis/ageofempires/Infobox/Person/Player/Custom.lua | Updates bottom content return annotation to Renderable?. |
| lua/wikis/commons/Widget/Match/Ticker/Wrapper.lua | New wrapper component to render match cards (vertical/horizontal). |
| lua/wikis/commons/Widget/Match/Ticker/HorizontalContainer.lua | New horizontal layout container for carousel + countdown switch. |
| lua/wikis/commons/Widget/Match/Ticker/FilterableContainer.lua | New extracted filterable ticker container (tabs + filter expansion attrs). |
| lua/wikis/commons/Widget/Match/Ticker/Container.lua | Deprecates old container module in favor of FilterableContainer. |
| lua/wikis/commons/MatchTicker/Custom.lua | Updates custom entry points to use new controller-style ticker call and sets header via args. |
| lua/wikis/commons/MatchTicker/Controller.lua | Refactors match ticker to functional controller (parse config, fetch matches, render via wrapper component). |
| lua/wikis/commons/Infobox/UnofficialWorldChampion.lua | Switches upcoming matches rendering to new MatchTicker controller call. |
| lua/wikis/commons/Infobox/Team.lua | Switches upcoming matches rendering to new MatchTicker controller call. |
| lua/wikis/commons/Infobox/Person.lua | Switches upcoming matches rendering to new MatchTicker controller call. |
| lua/wikis/commons/Infobox/League.lua | Switches upcoming matches rendering to new MatchTicker controller call. |
| lua/wikis/commons/HiddenDataBox.lua | Switches match ticker rendering to new MatchTicker controller call. |
Comments suppressed due to low confidence (3)
lua/wikis/commons/MatchTicker/Controller.lua:545
- Module now returns a plain table (MatchTickerController) but all updated call sites use
MatchTicker{...}/MatchTicker(args)(i.e. calling the module value). Without a__callmetamethod (or returning a function), this will throw “attempt to call a table value” at runtime.
lua/wikis/commons/MatchTicker/Controller.lua:240 if not type(matches[1]) == 'table' thenis a precedence bug and never evaluates as intended. Also, the previous implementation sorted + trimmed to limit and appliedadjustMatch; the refactor currently returns unsorted/unadjusted results and can change display order/side-swapping behavior.
lua/wikis/commons/MatchTicker/Controller.lua:372previousMatchWasTbdis module-level state used for filtering TBD matches; it can leak between multiple ticker invocations on the same page parse (or between different tickers on the same page), causing the first TBD match of a later ticker to be dropped unexpectedly.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
How did you test this change?